Skip to content

[STREAM-978] - Cleanup after failed ICE state.#326

Open
zservies wants to merge 2 commits intodevelopfrom
STREAM-978
Open

[STREAM-978] - Cleanup after failed ICE state.#326
zservies wants to merge 2 commits intodevelopfrom
STREAM-978

Conversation

@zservies
Copy link
Collaborator

@zservies zservies commented Nov 10, 2025

No description provided.

@zservies zservies requested a review from AidanWZ November 10, 2025 22:36
@zservies
Copy link
Collaborator Author

zservies commented Nov 10, 2025

I think we might already be cleaning up interruptions in genesys-cloud-media-session.ts https://github.com/purecloudlabs/genesys-cloud-streaming-client/blob/develop/src/types/genesys-cloud-media-session.ts#L227.

@zservies zservies added the bug-fix Fixes a bug. label Nov 11, 2025
hjon
hjon reviewed Nov 11, 2025
View reviewed changes
src/types/stanza-media-session.ts
candidatesReceivedFromPeer: this.iceCandidatesReceivedFromPeer
});
// Cleanup the session because failed is a state of finality.
this.end('failed-transport');
Copy link
Collaborator

@hjon hjon Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't have to do this, because stanza already takes care of that in ICESession and we ultimately extend that.

@hjon
Copy link
Collaborator

hjon commented Nov 11, 2025 •
edited
Loading

I think we might already be cleaning up interruptions in genesys-cloud-media-session.ts https://github.com/purecloudlabs/genesys-cloud-streaming-client/blob/develop/src/types/genesys-cloud-media-session.ts#L227.

I'll admit that I still don't feel super comfortable about the difference between connectionState and iceState, but it might still be worth checking when the iceState changes. I linked to this in the other comment, too, but stanza uses the combination of the two states to decide whether to end or just restart the ICE process: https://github.com/legastero/stanza/blob/master/src/jingle/ICESession.ts#L331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hjon hjon hjon left review comments

@patrickfeeney03 patrickfeeney03 patrickfeeney03 approved these changes

@maxwellmooney13 maxwellmooney13 maxwellmooney13 approved these changes

@AidanWZ AidanWZ Awaiting requested review from AidanWZ

Assignees

No one assigned

Labels

bug-fix Fixes a bug.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zservies @hjon @patrickfeeney03 @maxwellmooney13